Skip to content

fix(tools): resolve {{ENV_VAR}} references in user-only params for copilot tool executions#5679

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/copilot-env-var-resolution
Jul 14, 2026
Merged

fix(tools): resolve {{ENV_VAR}} references in user-only params for copilot tool executions#5679
waleedlatif1 merged 3 commits into
stagingfrom
fix/copilot-env-var-resolution

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes Chat-invoked integration tools with API-key auth failing with provider-side 401s (e.g. Sentry 401 Invalid token) — broken since the mothership tool-dispatch rewrite (improvement(mothership): restructured stream, tool structures, code typing, file write/patch/append tools, timing issues #4090) removed the orchestrator's env reference resolution
  • Chat agents can only pass {{VAR}} references (the workspace VFS exposes env var names, never values); the literal placeholder was being sent to the provider as the bearer token
  • Adds resolveCopilotEnvReferences in the tool executor: resolves whole-value {{VAR}} references on params declared visibility: 'user-only', gated to copilot executions only
  • Deliberately narrower than the removed deep resolver: LLM-writable params (urls, headers, bodies) never resolve, so references can't be used to extract secrets; embedded references are left untouched
  • Resolves against getEffectiveDecryptedEnv — the same personal+workspace merge workflow runs use, so Chat and canvas behave identically
  • Missing variables fail fast with an actionable error (naming the variable and parameter) before any request, instead of a confusing provider-side 401
  • Resolution mutates only the per-execution params copy, so decrypted values can't leak into failure logs or persisted chat state

Type of Change

  • Bug fix

Testing

  • 7 new tests: whole-value resolution, brace-whitespace trim, user-or-llm exclusion, embedded-reference exclusion, missing-var fail-fast, non-copilot passthrough, caller-params mutation guard
  • Full tools/index.test.ts suite (87 tests) passing; typecheck and lint clean
  • Root cause verified against production CloudWatch logs (three sentry_projects_list 401s via /api/mothership/chat with the literal placeholder in params)

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
fix(tools): resolve {{ENV_VAR}} references in user-only params for co…
4360677 
…pilot tool executions

Chat-invoked integration tools with API-key auth have been broken since the
mothership tool-dispatch rewrite (#4090) removed the orchestrator's env
reference resolution. Agents pass {{VAR}} references (the VFS exposes env var
names only), and the literal placeholder was sent to the provider, producing
auth failures like Sentry's 401 Invalid token.

Resolution is deliberately narrower than the removed deep resolver: only
whole-value references on params declared visibility user-only, gated to
copilot executions, resolving against the same personal+workspace env merge
workflow runs use. LLM-writable params (urls, headers, bodies) never resolve,
so references cannot be used to extract secrets. Missing variables fail fast
with an actionable error instead of a provider-side 401.
@vercel

vercel Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 14, 2026 11:33pm

Request Review

@cursor

cursor Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches secret resolution and decrypted env loading for copilot-only tool calls; scope is narrowed to user-only whole references with tests, but mishandling could leak or mis-resolve credentials.

Overview
Copilot Chat was sending literal {{VAR}} placeholders to integrations (e.g. API keys), causing provider 401s after env resolution was dropped from the copilot tool path.

This PR adds resolveCopilotEnvReferences in executeTool, run only when copilotToolExecution is set. It replaces whole-value {{ENV_VAR}} strings on params with visibility: 'user-only' using getEffectiveDecryptedEnv and the same resolveEnvVarReferences helper as workflow runs (allowEmbedded: false). LLM-writable params and embedded references are unchanged. Missing vars or missing user context fail before any HTTP call; resolution updates only the per-execution params copy.

Tests add a mock env tool and a suite covering resolution, exclusions, errors, non-copilot passthrough, and no mutation of caller params.

Reviewed by Cursor Bugbot for commit 728917c. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restores env variable reference handling for copilot tool executions. The main changes are:

  • Resolves whole-value {{ENV_VAR}} references in user-only tool params.
  • Keeps LLM-writable and embedded-reference params unresolved.
  • Fails before the provider request when user context or variables are missing.
  • Adds tests for resolution scope, missing context, workspace messaging, and mutation safety.

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Important Files Changed

Filename Overview
apps/sim/tools/index.ts Adds copilot-only env reference resolution for user-only params before hosted-key injection.
apps/sim/tools/index.test.ts Adds tests for env reference resolution, missing context errors, passthrough behavior, and mutation safety.

Reviews (2): Last reviewed commit: "fix(tools): fail fast on env references ..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Jul 14, 2026
View reviewed changes
Comment thread apps/sim/tools/index.ts
Comment thread apps/sim/tools/index.ts Outdated
@waleedlatif1
refactor(tools): delegate copilot env reference resolution to the sha…
b6f0314 
…red executor resolver

Replaces the hand-rolled exact-match regex with resolveEnvVarReferences
(allowEmbedded: false), the same resolver used by workflow runs, MCP config,
and webhooks — one set of reference semantics instead of two that can drift.
Behavior is identical; all existing tests pass unchanged.
@waleedlatif1
fix(tools): fail fast on env references without user context, clarify…
728917c 
… personal-only scope errors

Addresses review: a copilot execution missing userId now errors explicitly
instead of forwarding the literal placeholder upstream, and a missing-variable
error without a workspace context explains that only personal variables are
in scope there (matching workflow-run resolution semantics).
@waleedlatif1

waleedlatif1 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

waleedlatif1 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

cursor[bot]
cursor Bot reviewed Jul 14, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 728917c. Configure here.

@waleedlatif1 waleedlatif1 merged commit c203f51 into staging Jul 14, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/copilot-env-var-resolution branch July 14, 2026 23:38
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 15, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1